Wrap recipe errors with RecipeError to attribute failures to a nested recipe#7568
Closed
steve-aom-elliott wants to merge 3 commits intomainfrom
Closed
Wrap recipe errors with RecipeError to attribute failures to a nested recipe#7568steve-aom-elliott wants to merge 3 commits intomainfrom
steve-aom-elliott wants to merge 3 commits intomainfrom
Conversation
steve-aom-elliott
added
enhancement
… recipe The `Consumer<Throwable>` returned by `ExecutionContext#getOnError()` previously received the raw throwable from a failing recipe, leaving downstream tooling no way to tell which nested recipe (in a large declarative recipe with hundreds or thousands of children) actually produced the failure. Introduce `RecipeError` — a `RuntimeException` carrying the recipe path (root → leaf) and source path — and wrap throwables at the single chokepoint `RecipeRunCycle.handleError` and the run-timeout site before invoking the `onError` consumer. The `SourcesFileErrors` data table continues to receive the original sanitized stack trace, so attribution there is unchanged. Update `RewriteTest`'s default consumer to unwrap the new `RecipeError` before its `instanceof RecipeRunException` check.
steve-aom-elliott
force-pushed
the
feat/recipe-error-attribution
branch
from
648d385 to
8b3259c
Compare
…unException RewriteTest.defaultExecutionContext was passing the outer RecipeError to fail(), which made the resulting AssertionError's cause RecipeError instead of the underlying RecipeRunException. This regressed RepeatTest.repeatValidatesCursorIsPassed, which asserts that the cause is a RecipeRunException.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
Consumer<Throwable>returned byExecutionContext#getOnError()previously received the raw throwable from a failing recipe, leaving downstream tooling no way to tell which nested recipe — in a large declarative recipe with hundreds or thousands of children — actually produced the failure. The stack trace alone does not name the responsible recipe.This PR introduces
RecipeError, aRuntimeExceptionthat carries:List<String> recipeStack— names from root to leaf at the time of failureString sourcePath— the source file being processed (or"error during generation"for the scanning-generate path)Wrapping happens at the single chokepoint
RecipeRunCycle.handleErrorand at the per-cycle timeout site, immediately before invokingctx.getOnError().accept(...). All six existinghandleErrorcallsites pass the recipe stack already in scope (either the lambda parameter fromRecipeStack#reduceor the per-batch stack list from RPC batching), so no new bookkeeping is needed.The
SourcesFileErrorsdata table continues to receive the original sanitized stack trace viaExceptionUtils.sanitizeStackTrace, so attribution there is unchanged.RewriteTest's default error consumer is updated to unwrap aRecipeErrorbefore itsinstanceof RecipeRunExceptioncheck, so existing assertions keep working.Test plan
rewrite-corefull test suite passesRecipeSchedulerTestcover: leaf attribution, full nested-stack attribution through aDeclarativeRecipeparent, and generate-time error wrappingrewrite-test,rewrite-yaml,rewrite-properties,rewrite-json,rewrite-hclall compile/test cleanlyNotes
RewriteTestergonomics change.RecipeRunCycle#flushBatchcurrently attributes a batch failure to the first recipe in the batch regardless of which visitor failed remotely. Properly fixing that requires a newfailedIndexfield on the RPC error response and updates across the Java/.NET/Node RPC implementations — out of scope here.